Skip to content

add proxy corrections#17

Closed
andermi wants to merge 9 commits intombari-org:mainfrom
andermi:andermi/add_biolume_correction
Closed

add proxy corrections#17
andermi wants to merge 9 commits intombari-org:mainfrom
andermi:andermi/add_biolume_correction

Conversation

@andermi
Copy link
Member

@andermi andermi commented Apr 15, 2025

added proxy correction function to resample.py

addresses #5

andermi added 3 commits April 14, 2025 19:14
Signed-off-by: Michael Anderson <anderson@mbari.org>
Signed-off-by: Michael Anderson <anderson@mbari.org>
Signed-off-by: Michael Anderson <anderson@mbari.org>
@andermi
Copy link
Member Author

andermi commented Apr 15, 2025

I need to write a test to fit into your CI so I can tell it works

@andermi
Copy link
Member Author

andermi commented Apr 15, 2025

when I run the tests locally, I get this error:

>           assert nc_file.stat().st_size == EXPECTED_SIZE_LOCAL  # noqa: S101
E           AssertionError: assert 58930 == 58896
E            +  where 58930 = os.stat_result(st_mode=33204, st_ino=41460351, st_dev=66308, st_nlink=1, st_uid=11883, st_gid=11883, st_size=58930, st_atime=1744743887, st_mtime=1744744249, st_ctime=1744744249).st_size
E            +    where os.stat_result(st_mode=33204, st_ino=41460351, st_dev=66308, st_nlink=1, st_uid=11883, st_gid=11883, st_size=58930, st_atime=1744743887, st_mtime=1744744249, st_ctime=1744744249) = <bound method Path.stat of PosixPath('/home/anderson/sandbox/andermi_sandbox/plankton/auv-python/auv_data/i2map/missionnetcdfs/2018.348.01/i2map_2018.348.01_1S.nc')>()
E            +      where <bound method Path.stat of PosixPath('/home/anderson/sandbox/andermi_sandbox/plankton/auv-python/auv_data/i2map/missionnetcdfs/2018.348.01/i2map_2018.348.01_1S.nc')> = PosixPath('/home/anderson/sandbox/andermi_sandbox/plankton/auv-python/auv_data/i2map/missionnetcdfs/2018.348.01/i2map_2018.348.01_1S.nc').stat

src/data/test_process_i2map.py:43: AssertionError

@andermi
Copy link
Member Author

andermi commented Apr 15, 2025

@MBARIMike for some reason in the test: poetry run pytest, the biolume_proxy_--- fields that are supposed to be added by add_biolume_proxies are not present in df_r when it gets to me. Is this because there might not be any night time data in src/data/test_process_dorado.py?

614         if nighttime_bl_raw.empty:

…le from resampled_nc

Signed-off-by: Michael Anderson <anderson@mbari.org>
@MBARIMike
Copy link
Contributor

AssertionError: assert 58930 == 58896 can be expected when running on a different machine. See this comment.

@MBARIMike
Copy link
Contributor

I picked a really short mission for test_process_dorado.py so that it would run fast. Any mission containing nighttime data would be long as Dorado is usually deployed in the morning. Long missions take ~10 minutes to process. We should figure out another way to test the proxy calculations – maybe with a dozen or so data values, the same way it's done in test_ctd_proc.py.

@andermi
Copy link
Member Author

andermi commented May 1, 2025

How do I pass Monique's sample nc files to resample.py? I'm not sure what steps of process_dorado.py they need to pass through.

@MBARIMike
Copy link
Contributor

I realized that the troubleshooting process I use has not been documented. I made a start with TROUBLESHOOTING.md. I'm envisioning grabbing a few profiles from Monique's sample nc files to compare with data that's processed in resample.py. From this we can begin to create a test_proxy_correction.py file that can run in CI.

Signed-off-by: Michael Anderson <anderson@mbari.org>
@andermi
Copy link
Member Author

andermi commented May 13, 2025

@MBARIMike Will resample only ever process a single survey where profile_number will always be monotonically increasing? Before when processing multiple surveys at once, I had to generate unique profile numbers across all surveys.

@MBARIMike
Copy link
Contributor

I believe the answer is yes. The profile_number starts at 0 for each mission and increases monotonically as computed in this code.

Signed-off-by: Michael Anderson <anderson@mbari.org>
@andermi andermi marked this pull request as ready for review May 21, 2025 21:38
Signed-off-by: Michael Anderson <anderson@mbari.org>
@andermi
Copy link
Member Author

andermi commented May 21, 2025

@MBARIMike not sure why process_i2map is angry in CI. I didn't touch that file at all.

@MBARIMike
Copy link
Contributor

MBARIMike commented May 21, 2025

Yeah, it's unhappy with me too. I think it's safe to ignore that failing test with the changes in this PR.

@MBARIMike
Copy link
Contributor

Testing this PR on my machine using "args": ["--auv_name", "dorado", "--mission", "2020.337.00", "-v", "1"], in 4.0 - resample.py gives me:

TypeError: Resampler.correct_biolume_proxies.<locals>.<lambda>() got an unexpected keyword argument 'include_groups'

Doing a poetry update pandas upgraded from version = "2.1.4" to version = "2.2.3" and then the execution of completed, but with deprecation warnings, namely:

FutureWarning: 'S' is deprecated and will be removed in a future version, please use 's' instead.

The poetry update command updated the poetry.lock file. Please update your poetry.lock file and commit it to this PR.

@andermi
Copy link
Member Author

andermi commented May 23, 2025

Not sure you want my poetry.lock file. I'm running on python3.12

@andermi
Copy link
Member Author

andermi commented May 23, 2025

I can just remove the include_groups as it just silenced a warning I was getting

@MBARIMike
Copy link
Contributor

Removing the include_groups sounds good. I really need to update the Python version. I think I was stuck at 3.10 because of some problem with HoloViews. That's probably been solved now, but I need to check and then do some updates to prevent too much technical debt.

@andermi
Copy link
Member Author

andermi commented May 23, 2025

I could make another branch with my working python3.12 poetry env for you to try if you like

andermi added 2 commits May 23, 2025 15:08
Signed-off-by: Michael Anderson <anderson@mbari.org>
…dinos_threshold

Signed-off-by: Michael Anderson <anderson@mbari.org>
@MBARIMike
Copy link
Contributor

I needed to change how the processed files are copied to the archive, so will be adding a commit that should also fix the nit that's causing the lint test to fail.

@MBARIMike
Copy link
Contributor

Replaced by #24.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants